feat: nio async library#228
Conversation
|
neotest-dotnet: @Issafalcon |
Just a small change to adapt to the new async library in neotest. Depends on nvim-neotest/neotest#228
Just some small changes to adapt to the new async library in neotest. Depends on nvim-neotest/neotest#228
Just a small change to adapt to the new async library in neotest. Depends on nvim-neotest/neotest#228
|
Had to replace a |
|
I have a PR up for overseer that uses the new library and generally works, but it spits out this error: I've tried fiddling with the implementation and passing functions around, but I can't seem to find where this is coming from. This highlights my main issue with writing async code in most languages: stack traces lose information and it becomes a lot harder to debug. @rcarriga Do you have any tips for figuring out where this |
|
Thanks for the testing and updating everyone! I'd forgotten to check overseer, but great to see it updated too.
Providing stack traces is actually a big thing I tried to do in nio because I had the same experience of things just failing mysteriously 😅 The reason there's no trace here is because it's not nio that is catching the error, it is this code: Changing the code to use local ok, result = xpcall(comp[name], function (err)
log:error("Task %s dispatch %s.%s: %s\n%s", self.name, comp.name, name, err, debug.traceback(err, 2))
end, comp, self, ...)
if ok and result ~= nil then
table.insert(ret, result)
endThe error is happening because you're using Calling wrapped functions is supported to allow users to be able to use them in non-async contexts with the usual callback style without having to choose between an async version and synchronous version. This has the disadvantage of not giving a very helpful error because it's be up to the wrapped function to check for callback. Nio could check that a callback is provided in non-async contexts and error if it's not but that would prevent uses where a callback is optional. As a middleground we can log a warning to nio's log file. |
Just a small change to adapt to the new async library in neotest. Depends on nvim-neotest/neotest#228
|
Thanks for the help! The overseer PR is ready to go with full support for the new nio library. |
|
Hi @rcarriga - Thanks for the update. I've tested your changes on my end. All looks fine to me. The (few) tests I have still pass also. |
rcarriga
force-pushed
the
feat/nio-async-library
branch
from
500732c to
299101b
Compare
BREAKING CHANGE: Removes usage of plenary's async library which is not compatible with the new library.
Tasks without a callback that fail will now raise the error to the user to prevent silent failures. Consumers APIs now use nio.create to wrap async functionality so users can choose to wait for results with callbacks or in async contexts.
rcarriga
force-pushed
the
feat/nio-async-library
branch
from
299101b to
da4f02a
Compare
|
Brilliant, since we're looking good I'll get to merging this |
Tests were broken since we still using `require("plenary.async.tests")`.
This was no longer working after nvim-neotest/neotest#228
Per the comments in nvim-neotest/neotest#228, adapters using "plenary.async.tests" will have broken tests. This is remedied by using "neotest.async", which will import the nio library once 228 is merged. I've also switched from using TSInstallSync to TSUpdateSync for installing the rust parser in the makefile. TSUpdateSync will still install the parser if it does not exist but will not hang when running 'make test' on a system where it is already installed.
6 participants
Neotest is migrating away from using plenary's async library, and replacing it with a new library called
nio. There are several reasons for this change.Reasoning
Some major ones are:
library. Plenary has very few of either.
check source code for how to use it (e.g. some control objects use regular
.syntax such as
mpscwhereas others use:syntax such asSemaphore).apcallThis is not to say the plenary library has not been great and I absolutely appreaciate all the effort that went into it but the library is now seemingly no longer actively maintained (e.g. nvim-lua/plenary.nvim#293 and nvim-lua/plenary.nvim#298) and I believe there are significant enough changes to be made to justify packaging a new library.
The new library has a simple goal: providing common asynchronous primitives and asynchronous APIs for Neovim's core. It has been heavily influenced by the previous work for async IO in neovim including plenary and others. There are plans for Neovim to embed an async library within core which will provide a barebones core for libraries to share for compatibility (see neovim/neovim#19624). Nio aims to maintain compatibility with this library.
Features of
nioinclude:What does this mean for adapters?
I've been using the library embedded in nvim-dap-ui for a while now but neotest provides a better use case as the async usages are more complex and diverse. Right now I'll be leaving it embedded in neotest, with it being released as a separate plugin in future.
I've checked the source code all the adapters mentioned here (gotten from the neotest README) and most of them won't see any user facing issues as they are using
neotest.asyncwhich has been migrated to point tonio. All usage offn,apianduvwill work as before.Those adapters using
require("plenary.async.tests")will have broken tests, which can be fixed by switching torequire("nio").tests.neotest-jest and neotest-dotnet do use the
controlmodule which which has changed significantly. I will open PRs to make changes to use the new primitives.I'm planning on merging these changes relatively soon so please let me know of any issues but hopefully this will be a painless switch 😄